Skip to content

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Nov 6, 2023

This PR

  • resolves Add support for computing the hypotenuse via hypot #544 by adding a specification for computing the square root of the sum of squares.
  • follows IEEE 754 guidance that, if one of the operands is +-infinity, the result is infinity, regardless of whether the other operand is NaN. Intentionally, this deviates from a naive sqrt(x**2 + y**2) implementation.
  • only allows underflow when both arguments are subnormal and the correct result is also subnormal. This follows C99 and the POSIX standard and effectively prohibits a naive implementation. The entire purpose of this API is to avoid underflow/overflow during intermediate steps of computation and allowing a naive implementation defeats that purpose.
  • follows sqrt in requiring floating-point data types. The guidance uses "should", not "must", to allow implementations to support non-floating-point data types. The reason for the floating-point restriction is that the output data type must be a real-valued floating-point data type and promoting from, e.g., an integral to a floating-point data type is implementation-defined. For defined promotion semantics, a consumer should provide input arrays having floating-point data types.

@kgryte kgryte added API extension Adds new functions or objects to the API. topic: Complex Data Types Complex number data types. labels Nov 6, 2023
@kgryte kgryte added this to the v2023 milestone Nov 6, 2023
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, thanks @kgryte.

I plan to merge this soon unless there are more review comments.

@leofang
Copy link
Contributor

leofang commented Nov 16, 2023

I am a bit concerned about mentioning subnormal here. In CuPy we always set the compiler flag -ftz=true which would flush all denormals to zero. This helps significantly with floating point operations on GPU. Would this PR imply that we can no longer do this?

@leofang
Copy link
Contributor

leofang commented Feb 20, 2024

Q: I forgot what conclusion we had on this API, @kgryte do we plan to target v2023?

@kgryte
Copy link
Contributor Author

kgryte commented Feb 21, 2024

@leofang I've added a note concerning subnormal behavior and the fact that actual hardware support may vary. Does this allay your concerns?

Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@rgommers
Copy link
Member

Great, looks like everyone is happy now - let's hit the green button. Thanks @kgryte & reviewers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API extension Adds new functions or objects to the API. topic: Complex Data Types Complex number data types.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for computing the hypotenuse via hypot
4 participants